Refactoring auth module and tests #357
Conversation
omosola
force-pushed
the
refactor/auth
branch
4 times, most recently
from
516de10 to
09d0287
Compare
omosola
force-pushed
the
refactor/auth
branch
9 times, most recently
from
69034ba to
80e249a
Compare
omosola
force-pushed
the
refactor/auth
branch
2 times, most recently
from
1d21e48 to
9515813
Compare
josvandervelde
left a comment
josvandervelde
left a comment
There was a problem hiding this comment.
Thanks, Omosola! Some nice improvements. There are some problems that don't surface when using the unittests - let me know if I can be of any help.
server/user/views.py
Outdated
| session.commit() | ||
| return jsonify({ | ||
| "msg": "API Key updated", | ||
| "apikey": user.session_hash |
There was a problem hiding this comment.
+1 for other reviewers: adding the apikey here makes sure that the frontend can show the new api-key to the user.
|
|
||
| ctx.pop() | ||
|
|
||
|
|
There was a problem hiding this comment.
Not related to this PR, but the current tests seem to assume that the user sets the environment variable DO_SEND_MAIL=False.
Adding this code fixes that issue:
from server.public import views
@pytest.fixture(scope="session", autouse=True)
def set_env():
with patch.object(views, "DO_SEND_EMAIL", False) as _fixture:
yield _fixture
|
Thanks for the review @josvandervelde! A couple of changes are related to previous implementation decisions that I left alone for this PR since I wasn't sure about the reasons behind some of those choices. I didn't want to touch too much existing decision logic in this PR. My main focus for this PR was getting the user test suite running cleanly and finishing up unfinished implementations discovered through those updated tests (and aligning them with the old app's functionality). All good comments/points, though! I'll make those changes, but I might put some of them in a separate PR. |
…res more for test_reset_password
…efactor of Signup module
… password) across backend and frontend
omosola
force-pushed
the
refactor/auth
branch
from
9515813 to
903b94d
Compare
2 participants
Opening for discussion
Refactoring of (and some fixes in) the auth module, including tests.
Expanding test suite to cover more cases, and correcting existing tests.
Now with clean test suite runs. ✅
Added notes along the PR for main areas to focus on.